Skip to content

fix(search-notes): say that only titles were searched when the result is empty - #114

Merged
sweetrb merged 2 commits into
sweetrb:mainfrom
oliverames:fix/search-notes-title-only-hint
Jul 31, 2026
Merged

fix(search-notes): say that only titles were searched when the result is empty#114
sweetrb merged 2 commits into
sweetrb:mainfrom
oliverames:fix/search-notes-title-only-hint

Conversation

@oliverames

Copy link
Copy Markdown
Contributor

Description

search-notes matches note titles unless the caller passes searchContent: true. When a title-only search matches nothing, the response is a bare {"notes":[],"count":0} with nothing in it saying that bodies were never read.

I hit this as a caller. Asked to find notes on a topic, the natural first call is search-notes({query: "<term>"}). I made that call for several terms that appear many times in note bodies and got an empty result each time. The obvious reading of that response is "no such note exists." It is wrong, and nothing in the response says so. I only noticed because an unrelated query matched a note whose title happened to contain the term inside a longer word, which revealed that matching was title-only. After that I enumerated folders by hand to find notes I already knew were there.

That is a silent false negative, which is the worst failure mode for a search tool. It does not error, it confidently reports absence. On my own library the term because matches 0 titles and 62 bodies, so the empty result was hiding 62 notes.

This PR adds one hint to the empty result:

No notes found matching "because" in titles

ℹ️ Only note titles were searched, so a term that appears in note bodies would not match.
Retry with `searchContent: true` to search bodies instead.

Scope and cost:

  • The hint is built only when the result count is 0, so a successful search is byte-identical and no extra AppleScript work happens. There is no performance risk here.
  • searchContent still defaults to false, and no search semantics change.
  • The disclosure follows the mechanism already in the same registration block: a small pure helper returning a text fragment, the way describeSearchLimit discloses the applied result cap. New file src/utils/searchScope.ts with a sibling test, rather than widening searchLimit.ts, whose doc comment is specifically about the cap.

The tool description does mention searchContent, so this was never undocumented. But a description is not in front of the caller at the moment it is interpreting an empty result, and that is where the wrong conclusion gets drawn.

A question for you, deliberately not implemented here

While tracing this I found that searchContent: true searches bodies instead of titles, not in addition to them. searchNotes builds the whose clause as either name contains or body contains and never both (src/services/appleNotesManager.ts), so there is no way to get title-or-body matches in one call. The wording of my hint says "instead" for exactly that reason.

A "title" | "body" | "both" mode looks like the real fix, but it has whose-clause performance implications on a large library, and #100 already showed how tight the 30s budget is, so I did not want to bundle a guess about that into a disclosure PR. Two follow-on notes if you decide to pick it up:

  • The same false negative exists in mirror image today: searchContent: true with 0 results means titles were never searched. I deliberately left that case silent rather than expand this PR's scope. Say the word and I will make the hint symmetric.
  • Adding both would be a behavior change to an existing parameter, whereas a separate mode parameter could keep searchContent as a compatible alias.

Happy to send that as its own PR if you want it, in whatever shape you prefer. Your call on scope.

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactoring
  • Other (describe):

Testing

  • Tests pass locally (pnpm test) — 518 tests across 20 files, including 3 new cases in src/utils/searchScope.test.ts covering the hint on an empty title search, silence when titles matched, and silence when the caller already searched bodies
  • Linting passes (pnpm run lint)
  • Typecheck and formatting pass (pnpm run typecheck && pnpm run format:check)
  • Build succeeds (pnpm run build), and the committed bundle matches source (git diff --quiet build/ after a rebuild)

Also verified live, not just against mocks. I drove the built build/index.js over stdio against a real iCloud library and confirmed all three paths: a title-only search with no match now carries the hint, the same query with searchContent: true does not, and a search that finds titles is unchanged apart from the existing limit disclosure.

Checklist

  • I have read the CONTRIBUTING guidelines
  • My code follows the project's style
  • I have updated documentation if needed — CHANGELOG entry added; no README or SKILL.md change is needed, since the tool's documented contract is unchanged and only the empty-result text gains a line
  • If this PR changes shipped code: version bumped at least a patch + a CHANGELOG.md entry

One note on the version bump. I bumped to 2.6.11 and put the entry under [Unreleased], matching what #104 did for 2.6.10, rather than opening a ## [2.6.11] section. That keeps the existing unreleased items and this change filed together, since they will ship in the same release. If you would rather see a dated version heading, I am glad to restructure it.

`searchContent` defaults to false and the two modes are exclusive: the
`whose` clause is built as either `name contains` or `body contains`,
never both. So the natural first call, `search-notes({query})`, returned
a bare `{"notes":[],"count":0}` for a term appearing in many note
bodies, with nothing in the response saying bodies were never read.

That is a silent false negative. The tool does not error, it reports
absence, and a caller reasonably concludes the note does not exist. On a
real library the term "because" matches 0 titles and 62 bodies.

The empty result now carries a hint that only titles were searched and
that `searchContent: true` searches bodies instead, using the same
disclosure mechanism as the applied result cap. Scoped to the empty
result, so a successful search is unchanged and no extra AppleScript
work is done. The default stays false and no search semantics change.
@oliverames
oliverames marked this pull request as ready for review July 31, 2026 22:07
oliverames pushed a commit to oliverames/apple-notes-mcp that referenced this pull request Jul 31, 2026
CLAUDE.md read "Set `searchContent: true` to search note body, not just
titles", which describes an additive search. The modes are exclusive:
`searchNotes` builds the `whose` clause as either `name contains` or
`body contains`, never both, so bodies are searched instead of titles.

This is the same misconception the empty-result hint in this PR exists to
correct, stated in the file agents read before they call the tool, so
fixing only the runtime message would leave the cause in place.
README.md's parameter table was already accurate and is untouched.
@sweetrb
sweetrb merged commit 3b39af0 into sweetrb:main Jul 31, 2026
10 checks passed
@sweetrb

sweetrb commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Merged — thank you, @oliverames. This is a genuinely good catch and an unusually complete PR.

The bug you found is the worst kind: search-notes didn't error, it confidently reported absence. Your framing of it as a silent false negative is exactly right, and the concrete number sold it — because matching 0 titles and 62 bodies on your library makes the failure mode impossible to argue with. Scoping the hint to the empty result was the right call too: a successful search stays byte-identical and no extra AppleScript runs, so there's no cost to the common path.

Two things I especially appreciated:

  • You caught that CLAUDE.md stated the opposite of the actual behaviour. It read "search note body, not just titles", which describes an additive search — while searchNotes builds the whose clause as either name contains or body contains, never both. That wrong line was sitting in the file agents read before they call the tool, so it was actively teaching the misconception that caused the bug. Fixing the code without fixing that would have left the trap in place.
  • You rebuilt and committed build/index.js yourself. Fork PRs get no automatic bundle rebuild here (dependabot-rebuild.yml only fires for Dependabot), so this is the step that normally sends a PR back for another round. You also bumped the version across all six manifests and wrote the CHANGELOG entry — nothing was left for me to add.

Mirroring describeSearchLimit for describeSearchScope was the right shape as well; it keeps the two disclosure helpers symmetrical and independently testable, which your three test cases cover cleanly.

Shipping in v2.6.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants